fix(objectql): MetadataFacade object writes now reach the map its reads use (#6725) - #7211
Conversation
…ds use (#6725) `MetadataFacade.register('object', …)` wrote through `SchemaRegistry.registerItem`, into the generic `metadata` map. Every one of the facade's object reads resolves from `objectContributors`, which only `registerObject` populates: `getObject` goes straight there; `get('object', …)` and `exists` go via `registry.getItem`, which special-cases the object type back to `getObject`; `list`/`listNames` go via `registry.listItems`, which special-cases to `getAllObjects`. So an object written through the public facade was readable back through none of them — `register` resolved and every read answered `undefined`. `IMetadataService` declares `getObject(name)` ≡ `get('object', name)` and its own conformance test round-trips a `register('object', …)` through both members, so this was a shipped contract that could not work. Dormant in-tree only because nothing on `main` installs a `MetadataFacade` into the `metadata` slot. The write now performs both halves of the two-place object write the registry documents (`SchemaRegistry.unregisterObject`'s header; the in-tree precedent is `MetadataProtocol.applyObjectRegistryMutation`): `registerObject` for the contributor entry the reads resolve, plus the existing `registerItem` for the stored document. Both type spellings are covered, since both are special-cased on the read side. The contributor gets a COPY: `applyProtection` stamps in place and `applySystemFields` returns its input unchanged when there is nothing to inject, so a shared reference would have leaked a synthetic package id onto the stored document — what the "never invents a synthetic package id" pin forbids. That pin keeps its direct read of the generic map, because the stored document is what it was written to guard. A package-less object registers under the `'sys_metadata'` sentinel with `_provenance: 'org'`, so it cannot read as code-shipped. `unregister('object', …)` removes both halves too. Without that the fix would have re-opened #6808 from the other side: a removal that empties only the generic map leaves `getObject` — what the data plane dispatches on — serving a deleted object for the life of the process. Refs #6725, #6505, PR #6723, #6808, ADR-0010, ADR-0029. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0141cZum72My2vskaQSoQ1tZ
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 1 package(s): 15 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also reference the affected code. These are read-only:
|
github-actions
Bot
added
size/m
documentation
Reverse-verificationPre-fix Measured: Same 10, same 4. No deviations from the prediction. Fix restored → 14/14 pass. Tests
GatesEnumerated fresh from All 48 root gates: PASS. Three ( This diff does not reach One note on the enumeration: Generated by Claude Code |
2 participants
Closes #6725.
The defect
MetadataFacade.register('object', …)wrote throughSchemaRegistry.registerItem, which stores into the genericmetadatamap. Every one of the facade's object reads resolves fromobjectContributors, which onlyregisterObjectpopulates:getObject(name)registry.getObjectobjectContributorsget('object', name),exists('object', name)registry.getItem→ special-cases the object type back togetObjectobjectContributorslist('object'),listNames('object'),listObjects()registry.listItems→ special-cases togetAllObjectsobjectContributorsSo an object written through the public facade was readable back through none of them —
registerresolved successfully and every subsequent read answeredundefined/[].IMetadataService(@objectstack/spec/contracts) declaresgetObject(name)≡get('object', name), and its own conformance test round-trips aregister('object', …)through both members. This was a shipped contract that could not work. Dormant in-tree only because nothing onmaininstalls aMetadataFacadeinto themetadataslot — but the class is exported from this package's root andcoreentrypoints, so a downstream host that installs it got the split, including ObjectQL's ownbridgeObjectsToMetadataService, whose "already registered?" probe would never answer and so would re-register the full object set on every boot.Shape chosen, and the blast radius measured for it
The card priced three dispositions and pre-ruled none. Measurement:
facade.register('object', …)today? In-tree: only this package's own test file.new MetadataFacade(...)appears nowhere onmainoutside the two objectql test files. As published API: any host occupying themetadataslot — and the two in-tree occupants (MetadataManager,createMemoryMetadata) both round-trip correctly, so the facade is the odd one out, not the reference.registerObjectdo thatregisterItemdoes not? System-field injection,better-authapiMethodsreconciliation, ADR-0079 primary-title designation,__searchcompanion provisioning, the ADR-0029 single-owner guard, contributor merge + priority sort, merge-cache invalidation, and the_objectRevisionbump.registerItemitself the bug? No — and this is the part that decides the shape.registerItem's docblock does say "non-object metadata", butSchemaRegistry.unregisterObject's header (A deleted runtimeobjectis still served bySchemaRegistry.getObject— the registry heal reaches themetadatamap but neverobjectContributors#6808) states the actual invariant: "a runtime-authoredobjectis written into TWO places (metadata['object']viaregisterItemandobjectContributorsviaregisterObject)". The one in-tree precedent for this exact write,MetadataProtocol.applyObjectRegistryMutation, does both, withpackageId || 'sys_metadata'. The facade was performing half of a documented two-place write.So: the write now performs both halves, rather than moving to one of them.
Rejected, with reasons:
registry.metadata.get('object')— would editregistry.getObject, which the data plane dispatches on (assertObjectRegistered), and would make the reads answer the stored document where IMetadataService.getObject has no declared relationship to get('object', name), and its two implementations disagree #6505 / PR docs(spec): declare whatIMetadataService.getObjectanswers with (#6505) #6723 just documented them as answering the runtime-effective object. Wrong answer, much wider radius.'object'at the facade — would makeMetadataFacadenon-conformant with the interface it is documented to implement, and would turn a silent no-op into a throw inside bothbridgeObjectsToMetadataServiceloops for any host that installs it.Details that are load-bearing
applyProtectionstamps_packageId/_provenancein place, andapplySystemFieldsreturns its input unchanged on the no-injection path (systemFields: false,managedBy: 'better-auth',sys_*). A shared reference would therefore have written a synthetic package id onto the generic-map entry — exactly what the provenance pin forbids. Pinned by a new test that registers asystemFields: falseobject, i.e. the aliasing path.'sys_metadata'sentinel with_provenance: 'org'— both of whichgetArtifactItem/isArtifactBackedexclude, so it cannot read as code-shipped (the cloud#970 misclassification). Without the explicit'org',applyProtectionwould default the copy to_provenance: 'package'. An object carrying a real_packageIdregisters under it and keeps'package'.'object'and'objects'are both special-cased on the read side, so both are covered on the write side.What happened to the provenance test
"never invents a synthetic package id for object registrations"is unchanged and still in place, still reading(registry as any).metadata.get('object')directly. That direct read remains the right instrument: the pin is about the stored document, and the object reads answer the contributor copy — which now exists and deliberately does carry the sentinel. Reading it throughget('object', …)would have silently retargeted the assertion. A comment on the test now says so, and a new sibling test pins the aliasing hazard the copy exists to prevent.Scope note:
unregisterunregister('object', name)now removes both halves too. This is not scope creep — without it the fix would have introduced #6808's shape from the other side: a removal that empties only the generic map leavesgetObject, which the data plane dispatches on, serving a deleted object for the life of the process.registry.unregisterObject(name)(the #6808 verb) is idempotent and refuses, per ADR-0029, an object still extended by another package.Behaviour changes a caller can observe
register('object', …)can now throw where it previously succeeded and did nothing: claiming an object another package owns is refused (ADR-0029).unregister('object', …)can throw for an object still extended by another package.Tests
packages/objectql/src/metadata-facade.test.ts— 11 new cases, 14 total: round-trip throughgetObjectandget(reference-identical, with an anti-vacuitytoBeDefined); the enumeration members; the plural spelling; the runtime-effective shape on a multi-tenant registry; sentinel-not-artifact and real-package-id provenance; idempotent re-registration; the ADR-0029 refusal writing nothing; removal from both places; idempotent removal of an absent object.Reverse-verified — pre-fix
metadata-facade.tsrestored under the new tests, red/green predicted per case before running. See the comment below.Refs #6725, #6505 / PR #6723, #6853, #6808, ADR-0010, ADR-0029.
Generated by Claude Code